Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/delete draft projec #4793

Merged
merged 17 commits into from
Oct 1, 2024
Merged

Feat/delete draft projec #4793

merged 17 commits into from
Oct 1, 2024

Conversation

MohammadPCh
Copy link
Collaborator

@MohammadPCh MohammadPCh commented Sep 28, 2024

Summary by CodeRabbit

Release Notes

  • New Features

    • Introduced a new GraphQL mutation DELETE_DRAFT_PROJECT for deleting draft projects.
    • Added a refetchProjects prop to the DeleteProjectModal for refreshing project lists after deletion.
    • New constants for user projects pagination and ordering have been defined.
    • A new function fetchUserProjects to retrieve user projects with pagination and ordering.
  • Improvements

    • Enhanced type safety in Apollo Client setup.
    • Refactored ProfileProjectsTab to utilize react-query for improved data fetching.
  • Bug Fixes

    • Updated import paths for EOrderBy and IOrder to streamline component dependencies.
  • Chores

    • Added development dependency @tanstack/react-query-devtools for improved development experience.

Copy link

vercel bot commented Sep 28, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
giveth-dapps-v2 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 28, 2024 10:16am

@MohammadPCh MohammadPCh marked this pull request as ready for review September 28, 2024 09:35
Copy link
Contributor

coderabbitai bot commented Sep 28, 2024

Walkthrough

The pull request introduces several changes across multiple files, primarily focusing on enhancing type safety in TypeScript, adding a new GraphQL mutation for deleting draft projects, and restructuring imports for better organization. It also integrates the @tanstack/react-query library for data fetching in some components, replacing Apollo Client's previous approach. Additionally, new constants and functions are defined to manage user projects more effectively.

Changes

File Change Summary
package.json Added new development dependency: @tanstack/react-query-devtools version ^5.58.0.
src/apollo/apolloClient.ts Enhanced type safety by updating variable and function parameter types; defined return types.
src/apollo/gql/gqlProjects.ts Added new GraphQL mutation DELETE_DRAFT_PROJECT accepting projectId as an argument.
src/components/views/project/projectDonations/ProjectRecurringDonationTable.tsx Updated import path for EOrderBy enumeration.
src/components/views/userProfile/UserProfile.view.tsx Removed EOrderBy enum and IOrder interface for simplification.
src/components/views/userProfile/donationsTab/oneTimeTab/OneTimeDonationsTable.tsx Updated import paths for EOrderBy and IOrder.
src/components/views/userProfile/donationsTab/oneTimeTab/OneTimeTab.tsx Changed import paths for IOrder and EOrderBy.
src/components/views/userProfile/projectsTab/DeleteProjectModal.tsx Added functionality for project deletion, including a new prop for refetching projects.
src/components/views/userProfile/projectsTab/ProfileProjectsTab.tsx Refactored to use react-query for data fetching instead of Apollo Client.
src/components/views/userProfile/projectsTab/ProjectItem.tsx Updated IProjectItem interface and JSX structure, added refetchProjects prop.
src/components/views/userProfile/projectsTab/constants.ts Introduced constants for user projects per page and ordering criteria.
src/components/views/userProfile/projectsTab/services.ts Added fetchUserProjects function for retrieving user projects with pagination and ordering.
src/components/views/userProfile/projectsTab/type.ts Added EOrderBy enum and IOrder interface for project ordering.

Possibly related PRs

Suggested reviewers

  • kkatusic
  • RamRamez
  • divine-comedian

Poem

🐰 In the garden of code, changes bloom bright,
With queries and types, we take flight.
A project to delete, with just a small click,
Refetching our data, oh, what a neat trick!
So hop along, friends, let’s celebrate cheer,
For a cleaner, safer codebase is here! 🌼


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between c7902d6 and b8891aa.

📒 Files selected for processing (2)
  • src/components/views/userProfile/projectsTab/DeleteProjectModal.tsx (3 hunks)
  • src/components/views/userProfile/projectsTab/ProfileProjectsTab.tsx (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/components/views/userProfile/projectsTab/DeleteProjectModal.tsx
  • src/components/views/userProfile/projectsTab/ProfileProjectsTab.tsx

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11

🧹 Outside diff range and nitpick comments (13)
src/components/views/userProfile/projectsTab/constants.ts (1)

6-9: LGTM: Well-structured constant with good use of enums. Consider adding a type annotation.

The projectsOrder constant is well-defined and uses the imported enums correctly. Ordering projects by creation date in descending order is a logical default.

For improved readability and type safety, consider adding a type annotation to the projectsOrder constant. Here's a suggested improvement:

-export const projectsOrder = {
+export const projectsOrder: { by: EOrderBy; direction: EDirection } = {
 	by: EOrderBy.CreationDate,
 	direction: EDirection.DESC,
 };
src/components/views/userProfile/projectsTab/type.ts (3)

11-16: LGTM: EOrderBy enum is well-defined.

The EOrderBy enum provides a clear set of options for ordering criteria, which is useful for maintaining type safety when implementing sorting functionality. The naming convention is consistent, and the members cover common sorting criteria for project-related data.

Consider adding a brief comment above the enum to describe its purpose and usage context. For example:

/**
 * Enum representing the available ordering criteria for projects.
 * Used in sorting and filtering operations.
 */
export enum EOrderBy {
  // ... (existing members)
}

18-21: LGTM: IOrder interface is well-structured.

The IOrder interface effectively combines the EOrderBy enum with the imported EDirection enum, providing a flexible and type-safe way to specify ordering criteria. The naming conventions follow TypeScript best practices.

Consider adding a brief comment above the interface to describe its purpose and usage context. For example:

/**
 * Interface representing the structure for specifying order criteria.
 * Used for sorting operations in project listings.
 */
export interface IOrder {
  // ... (existing properties)
}

1-21: Overall, the changes enhance type safety and project management capabilities.

The additions of EOrderBy enum and IOrder interface provide a structured and type-safe way to handle project ordering, which aligns well with the PR objective of implementing features related to project management. These changes will likely improve code maintainability and reduce potential runtime errors related to ordering operations.

As the project grows, consider creating a separate file for shared enums and interfaces if they are used across multiple components. This can help in maintaining a clean and modular code structure.

package.json (1)

81-81: Approve the addition of React Query devtools and suggest updating the main library.

The addition of @tanstack/react-query-devtools as a dev dependency is appropriate and will enhance the development experience. The version ^5.58.0 is compatible with the current @tanstack/react-query version.

Consider updating the main @tanstack/react-query library to match the devtools version:

- "@tanstack/react-query": "^5.45.1",
+ "@tanstack/react-query": "^5.58.0",

This will ensure you have the latest features and bug fixes in both the main library and the devtools.

src/components/views/userProfile/projectsTab/ProjectItem.tsx (4)

29-29: Approved: New refetchProjects property added to IProjectItem interface

The addition of the refetchProjects property is a good improvement, allowing the component to trigger a refresh of the projects list. This is particularly useful for maintaining data consistency after operations like deleting a project.

Consider using a more specific type for refetchProjects if possible:

refetchProjects: () => Promise<void>;

This assumes the refetch operation is asynchronous, which is typical for data fetching operations. If it's not asynchronous, the current type is fine.


Line range hint 47-62: Approved: Improved structure with new Header and Title components

The introduction of the Header component and the replacement of H2 with Title improve the structure and potentially the styling of the project item. These changes should lead to better maintainability and consistency across the application.

To ensure proper accessibility, consider adding an aria-label to the Link component:

<Link href={`/project/${project.slug}`} aria-label={`View project: ${project.title}`}>
  <Title>{project.title}</Title>
</Link>

This will provide more context for screen reader users.


159-162: Approved: New Header styled component added

The addition of the Header styled component with max-width: 100% and overflow: hidden is a good approach to handle potentially long content within the project header. This ensures that the layout remains consistent across different project items.

Consider making the max-width property configurable:

const Header = styled.div<{ $maxWidth?: string }>`
  max-width: ${props => props.$maxWidth || '100%'};
  overflow: hidden;
`;

This allows for more flexibility if you need to adjust the max-width in different contexts.


164-166: Approved with suggestion: New Title styled component added

The new Title styled component effectively handles long project titles by using text-overflow: ellipsis and overflow: hidden. This ensures a consistent layout while indicating to users that there's more text if the title is too long to display fully.

To improve accessibility and user experience, consider adding a title attribute to show the full text on hover:

const Title = styled(H2)`
  text-overflow: ellipsis;
  overflow: hidden;
  white-space: nowrap;  // Add this to ensure text doesn't wrap
`;

// In the JSX:
<Title title={project.title}>{project.title}</Title>

This allows users to see the full title by hovering over it, which is especially useful for screen readers and when titles are truncated.

src/apollo/gql/gqlProjects.ts (1)

807-812: LGTM! Consider specifying the return type for clarity.

The new DELETE_DRAFT_PROJECT mutation aligns well with the PR objective of implementing a feature to delete draft projects. The mutation signature is consistent with other mutations in the file.

For improved clarity and consistency, consider explicitly specifying the return type of the mutation. You can modify the mutation as follows:

 export const DELETE_DRAFT_PROJECT = gql`
 	mutation ($projectId: Float!) {
-		deleteDraftProject(projectId: $projectId)
+		deleteDraftProject(projectId: $projectId): Boolean!
 	}
 `;

This change makes it clear that the mutation returns a boolean value, indicating the success or failure of the operation.

src/components/views/userProfile/projectsTab/DeleteProjectModal.tsx (1)

46-48: Remove unnecessary 'async' keyword from 'handleRemoveProject'

The handleRemoveProject function is declared as async but does not contain any await expressions. Removing the async keyword can simplify the code.

Apply this diff to remove the unnecessary async keyword:

- const handleRemoveProject = async () => {
+ const handleRemoveProject = () => {
    deleteProject(Number(project.id));
  };
src/components/views/userProfile/projectsTab/ProfileProjectsTab.tsx (1)

51-51: Consider handling errors from useQuery

The component currently handles loading and empty states but does not account for potential errors during data fetching. It would improve user experience to check for isError from useQuery and display an appropriate error message when an error occurs.

Suggestion:

Add a check for isError and render an error message:

{isError && <ErrorMessage />}

Also applies to: 84-84

src/apollo/apolloClient.ts (1)

223-223: Consider More Specific Types for arrayMerge Function

Using any[] for destinationArray and sourceArray in arrayMerge provides flexibility but may reduce type safety. Consider defining more specific types if possible.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between dd33339 and c7902d6.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (13)
  • package.json (1 hunks)
  • src/apollo/apolloClient.ts (9 hunks)
  • src/apollo/gql/gqlProjects.ts (1 hunks)
  • src/components/views/project/projectDonations/ProjectRecurringDonationTable.tsx (1 hunks)
  • src/components/views/userProfile/UserProfile.view.tsx (0 hunks)
  • src/components/views/userProfile/donationsTab/oneTimeTab/OneTimeDonationsTable.tsx (1 hunks)
  • src/components/views/userProfile/donationsTab/oneTimeTab/OneTimeTab.tsx (1 hunks)
  • src/components/views/userProfile/projectsTab/DeleteProjectModal.tsx (3 hunks)
  • src/components/views/userProfile/projectsTab/ProfileProjectsTab.tsx (3 hunks)
  • src/components/views/userProfile/projectsTab/ProjectItem.tsx (5 hunks)
  • src/components/views/userProfile/projectsTab/constants.ts (1 hunks)
  • src/components/views/userProfile/projectsTab/services.ts (1 hunks)
  • src/components/views/userProfile/projectsTab/type.ts (1 hunks)
💤 Files not reviewed due to no reviewable changes (1)
  • src/components/views/userProfile/UserProfile.view.tsx
🔇 Additional comments (28)
src/components/views/userProfile/projectsTab/constants.ts (2)

1-2: LGTM: Imports are well-organized and follow good practices.

The imports are appropriately chosen for the constants defined in this file. The use of path alias ('@/') and local type imports ('./type') demonstrates good organization and maintainability.


4-4: LGTM: Clear constant definition. Verify the pagination value.

The userProjectsPerPage constant is well-named and exported correctly. The value of 12 seems reasonable for pagination.

Please verify that 12 projects per page aligns with the UI design and performance requirements. You can run the following script to check if this value is used consistently across the project:

src/components/views/userProfile/projectsTab/type.ts (1)

1-1: LGTM: Import statement is appropriate.

The import of EDirection is correctly used in the IOrder interface, maintaining type safety and consistency.

src/components/views/userProfile/projectsTab/services.ts (2)

1-5: LGTM: Imports are appropriate and consistent.

The imports are relevant to the function's purpose and follow a consistent style. They include necessary dependencies such as the Apollo client, types, GraphQL query, and constants.


7-11: LGTM: Function signature is well-defined and typed.

The fetchUserProjects function signature is clear, concise, and properly typed. It correctly uses async and includes appropriate parameters for fetching user projects with pagination and ordering capabilities.

src/components/views/userProfile/projectsTab/ProjectItem.tsx (2)

146-146: Approved: refetchProjects prop added to DeleteProjectModal

The addition of the refetchProjects prop to the DeleteProjectModal component is a good improvement. It allows the modal to trigger a refresh of the projects list after a project is deleted, ensuring that the UI stays in sync with the backend data.


156-157: Approved with query: Added overflow: hidden to ProjectContainer

The addition of overflow: hidden to the ProjectContainer styled component can help maintain a consistent layout by preventing content from spilling outside the container.

Could you please clarify the specific reason for adding this property? It's important to ensure that it doesn't unintentionally hide any important content. If there was a particular layout issue this is addressing, it would be helpful to document it in a comment.

src/components/views/userProfile/donationsTab/oneTimeTab/OneTimeDonationsTable.tsx (1)

26-26: LGTM! Verify import consistency across the project.

The update to the import statement for EOrderBy and IOrder is a good change, potentially centralizing these types for better maintainability. This change doesn't affect the component's functionality as the type names remain the same.

To ensure this change is consistent across the project, please run the following script:

This script will help identify any inconsistencies in the usage of these types across the project.

src/components/views/project/projectDonations/ProjectRecurringDonationTable.tsx (2)

Line range hint 228-228: Verify the usage of EOrderBy.TokenAmount

There seems to be an inconsistency in the usage of EOrderBy. The component uses EOrderBy.TokenAmount, but the local enum RecurringDonationSortField defines flowRate instead of TokenAmount. This might lead to a type error or unexpected behavior.

Please verify that EOrderBy.TokenAmount is the correct enum value to use here. If not, consider updating it to match the RecurringDonationSortField enum. Run the following script to check the definition of EOrderBy:

#!/bin/bash
# Description: Check the definition of EOrderBy

# Test: Display the contents of the file containing EOrderBy
cat src/components/views/userProfile/projectsTab/type.ts

# Test: Search for TokenAmount in EOrderBy
ast-grep --lang typescript --pattern 'enum EOrderBy { $$$TokenAmount$$$ }' src/components/views/userProfile/projectsTab/type.ts

If TokenAmount is not found in EOrderBy, consider updating the SortIcon usage to:

<SortIcon order={order} title={RecurringDonationSortField.flowRate} />

34-34: LGTM! Verify the new import path.

The import path for EOrderBy has been updated, which appears to be part of a project structure refactoring. This change doesn't affect the component's functionality as the usage of EOrderBy remains the same.

To ensure the new import path is correct, please run the following script:

This script will verify that EOrderBy is exported from the new file and that there are no remaining imports from the old path.

src/components/views/userProfile/projectsTab/DeleteProjectModal.tsx (1)

39-41: Verify the necessity of refetching user data after project deletion

After deleting the project, the code refetches the user's projects and dispatches fetchUserByAddress. Please verify if both actions are required or if refetching the projects suffices to update the user interface.

Run the following script to check where fetchUserByAddress impacts the UI:

This will help determine if re-dispatching fetchUserByAddress is essential after deleting a project.

✅ Verification successful

Refetching user data after project deletion is necessary

After reviewing the codebase, dispatching fetchUserByAddress is required to ensure that all user-related data is up-to-date. Therefore, both refetchProjects() and dispatch(fetchUserByAddress(walletAddress)) are necessary to maintain the user interface's consistency.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find components that rely on the user's data fetched by 'fetchUserByAddress'.

# Test: Search for components using 'fetchUserByAddress'. Expect: Identify if re-dispatching is necessary.
rg --type tsx "fetchUserByAddress"

Length of output: 62


Script:

#!/bin/bash
# Description: Find components that rely on the user's data fetched by 'fetchUserByAddress' within .tsx files.

# Find all .tsx files and search for 'fetchUserByAddress'
fd --extension tsx --type file | xargs rg "fetchUserByAddress"

Length of output: 1229

src/components/views/userProfile/projectsTab/ProfileProjectsTab.tsx (7)

2-2: Approved addition of useState import

The import of useState from 'react' is appropriate for managing the page state within the component.


5-5: Integration of useQuery from '@tanstack/react-query'

Adopting useQuery from '@tanstack/react-query' enhances data fetching and state management, aligning with best practices.


15-16: Added necessary imports for data fetching functions and constants

The imports of fetchUserProjects, projectsOrder, and userProjectsPerPage are correctly introduced and used appropriately in the component.


67-73: Approved rendering of project items for the user's account

Rendering ProjectItem components for the user's own projects is appropriate and correctly implemented.


77-77: Approved rendering of project cards for other users

Mapping over data?.projects to render ProjectCard components displays other users' projects effectively.


88-88: Approved usage of totalCount in Pagination component

Passing data?.totalCount || 0 to the Pagination component ensures it functions correctly even if data is undefined.


90-90: Approved usage of userProjectsPerPage constant

Using userProjectsPerPage for itemPerPage improves consistency and maintainability across the application.

src/apollo/apolloClient.ts (10)

2-7: Improved Type Safety with Explicit Imports

Good job on explicitly importing ApolloClient, InMemoryCache, ApolloLink, and NormalizedCacheObject from @apollo/client. This enhances type safety and code clarity.


22-22: Explicitly Typing apolloClient Enhances Clarity

Defining apolloClient as ApolloClient<NormalizedCacheObject> | undefined improves type safety and makes the code more self-documenting.


29-29: Refined Parameter Type for parseHeaders Function

Updating the rawHeaders parameter type from any to string in the parseHeaders function enhances type safety and reduces the risk of runtime errors.


34-34: Explicit Typing of line in forEach Loop

Specifying line as string in the forEach loop improves type checking and ensures that string methods are safely applied.


36-36: Safe Handling of undefined with Optional Chaining

Using optional chaining on parts.shift() safely handles potential undefined values before calling .trim(), preventing possible runtime errors.


59-63: Resolved TypeScript Error by Casting xhr

Casting xhr to XMLHttpRequest when accessing responseText fixes the TypeScript error, ensuring proper type usage and access to the responseText property.


97-98: Explicit Return Type for createApolloClient Function

Defining the return type as ApolloClient<NormalizedCacheObject> for the createApolloClient function enhances type safety and clarity.


208-211: Improved Type Annotations in initializeApollo Function

Specifying the initialState parameter type as any and the return type as ApolloClient<NormalizedCacheObject> improves type clarity and prevents potential type-related issues.


242-246: Explicit Typing of Parameters in addApolloState Function

Specifying the client parameter as ApolloClient<NormalizedCacheObject> enhances type safety and makes the function's contract clearer.


261-262: Explicitly Typed Exported client Instance

Defining the exported client with the explicit type ApolloClient<NormalizedCacheObject> improves the module's interface and ensures consistent usage across the codebase.

Copy link
Collaborator

@Meriem-BM Meriem-BM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! thanks @MohammadPCh

Copy link
Collaborator

@kkatusic kkatusic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ;), thx @MohammadPCh

@MohammadPCh MohammadPCh merged commit 14c4b07 into develop Oct 1, 2024
3 checks passed
@MohammadPCh MohammadPCh deleted the feat/delete-draft-projec branch October 1, 2024 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: QA
Development

Successfully merging this pull request may close these issues.

3 participants